Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(core): Remove client-oauth from oauth token's callback flow #5286

Closed
wants to merge 16 commits into from
Closed

refactor(core): Remove client-oauth from oauth token's callback flow #5286

wants to merge 16 commits into from

Conversation

agobrech
Copy link
Contributor

@agobrech agobrech commented Jan 30, 2023 •
edited by maspio
Loading

Fixes N8N-3921

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jan 30, 2023
@agobrech agobrech changed the title N8n 3921 switch client oauth2 for own helpers core: Remove client-oauth2 library from the exchange token Mar 1, 2023
@agobrech agobrech changed the title core: Remove client-oauth2 library from the exchange token core: Remove client-oauth from oauth token's callback flow Mar 2, 2023
@agobrech agobrech marked this pull request as ready for review March 2, 2023 17:06
@agobrech agobrech changed the title core: Remove client-oauth from oauth token's callback flow refactor(core): Remove client-oauth from oauth token's callback flow Mar 2, 2023
@github-actions
Copy link
Contributor

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

  • If fixing bug, added test to cover scenario.
  • If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

  • Added unit tests to cover new or updated functionality.

Make sure to check off this list before asking for review.

@netroy
Copy link
Member

netroy commented Mar 10, 2023 •
edited
Loading

Somehow this PR skipped my attention.
I have a PR on our client-oauth fork to convert the code to TS, and to remove the bits that we never use.
Should we try to port over that code into this repo.
That way we can switch to using the same http request mechanisms for oauth2 that we use for the rest of the code. That would enable us to do things like allowing self-signed certificates.

@codecov
Copy link

codecov bot commented Mar 15, 2023 •
edited
Loading

Codecov Report

Patch coverage: 21.11% and no project coverage change.

Comparison is base (39c871d) 13.03% compared to head (ebfb900) 13.04%.

❗ Current head ebfb900 differs from pull request most recent head 9c75e1d. Consider uploading reports for the commit 9c75e1d to get more accurate results

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #5286   +/-   ##
=======================================
  Coverage   13.03%   13.04%           
=======================================
  Files        2454     2455    +1     
  Lines      112599   112665   +66     
  Branches    17491    17507   +16     
=======================================
+ Hits        14679    14696   +17     
- Misses      97420    97469   +49     
  Partials      500      500
Impacted Files Coverage Δ
packages/editor-ui/src/stores/nodeCreator.ts 16.12% <0.00%> (-0.09%) ⬇️
.../nodes-base/nodes/Metabase/QuestionsDescription.ts 0.00% <0.00%> (ø)
...ackages/nodes-base/nodes/OpenAi/ChatDescription.ts 0.00% <ø> (ø)
...ckages/cli/src/credentials/oauth2Credential.api.ts 19.53% <14.28%> (ø)
...cli/src/credentials/credentials.helperFunctions.ts 17.74% <17.74%> (ø)
...s-base/nodes/SplitInBatches/SplitInBatches.node.ts 84.61% <100.00%> (+8.14%) ⬆️

... and 4 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@netroy
Copy link
Member

netroy commented May 16, 2023

Superseded by #6056

@netroy netroy closed this May 16, 2023
@netroy netroy deleted the n8n-3921-switch-client-oauth2-for-own-helpers branch May 16, 2023 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@agobrech @netroy @maspio